Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

alloc: update memory allocation pointer type from NonZeroUsize to NonNull<u8> in ByteAllocator trait #115

Merged
merged 3 commits into from
Aug 23, 2023

Conversation

pymongo
Copy link
Contributor

@pymongo pymongo commented Aug 23, 2023

This pull request updates the memory allocation pointer type from NonZeroUsize to NonNull in the ByteAllocator trait of the Rust open-source operating system project.

Motivation/Improvement Reason:

  1. The NonZeroUsize type can represent memory page numbers, allocation sizes, or offsets, which can be ambiguous in terms of semantics. Using NonNull provides a more accurate representation of returning a pointer to a memory segment.
  2. Previously, both the tlsf and buddy libraries required conversion from NonZeroUsize to NonNull. After this change, only the slab library needs to perform the conversion.
  3. The Rust standard library's core::alloc::Alloc also uses NonNull.

This PR modifies the ByteAllocator trait and its implementations to use NonNull as the memory allocation pointer type. The changes include updating the return type of the alloc method to AllocResult<NonNull>, and the parameter type of the dealloc method to NonNull

@equation314
Copy link
Member

Thanks! Please fix CI.

@equation314
Copy link
Member

After using NonNull<u8> instead of NonZeroUsize, the crate attributes #![feature(strict_provenance)] can be removed. Could you find all the attributes and remove them?

@@ -4,13 +4,13 @@ cfg_alloc! {

pub fn ax_alloc(layout: Layout) -> Option<NonNull<u8>> {
if let Ok(vaddr) = axalloc::global_allocator().alloc(layout) {
Some(unsafe { NonNull::new_unchecked(vaddr.get() as _) })
Some(vaddr)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These lines can be simplified to axalloc::global_allocator().alloc(layout).ok()

@equation314 equation314 merged commit e526753 into arceos-org:main Aug 23, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants